Skip to content

Add support for remote_indicies to elasticstack_elasticsearch_security_role & elasticstack_kibana_security_role#723

Merged
tobio merged 14 commits into
elastic:mainfrom
mholttech:722-add-remote-indices
Aug 26, 2024
Merged

Add support for remote_indicies to elasticstack_elasticsearch_security_role & elasticstack_kibana_security_role#723
tobio merged 14 commits into
elastic:mainfrom
mholttech:722-add-remote-indices

Conversation

@mholttech
Copy link
Copy Markdown
Contributor

@mholttech mholttech commented Aug 21, 2024

resolves #722 by adding remote_indices to elasticstack_kibana_security_role & elasticstack_elasticsearch_security_role

@cla-checker-service
Copy link
Copy Markdown

cla-checker-service Bot commented Aug 21, 2024

💚 CLA has been signed

@mholttech mholttech force-pushed the 722-add-remote-indices branch from 71d3232 to 9312ea2 Compare August 21, 2024 00:29
Comment thread internal/kibana/role.go
@tobio
Copy link
Copy Markdown
Member

tobio commented Aug 21, 2024

It looks like the build is failing because the committed docs don't match what would be generated. Are you able to run make docs-generate and commit the result?

Copy link
Copy Markdown
Member

@tobio tobio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The linked issue talks about the elasticsearch role resource (which is here). I'm not sure if adding it to the Kibana role is sufficient for you though?

Comment thread internal/kibana/role.go
@mholttech
Copy link
Copy Markdown
Contributor Author

Good catch. I didn't realize there was two seperate resources. Looks like the api for elasticstack_kibana_security_role is in preview but the work done is still valid. I'll expand the PR to include the intended resource and regenerate the docs.

@mholttech mholttech changed the title Add remote indices feature WIP: Add remote indices feature Aug 21, 2024
@mholttech
Copy link
Copy Markdown
Contributor Author

Everything should be in place for the elasticstack_kibana_security_role now. Still working on the elasticstack_elasticsearch_security_role

@mholttech
Copy link
Copy Markdown
Contributor Author

forgot about TestAccDataSourceKibanaSecurityRole, i've commented out the remote_indices for now, will add the version constraints tomorrow.

@mholttech mholttech force-pushed the 722-add-remote-indices branch from 27d2686 to 952183e Compare August 22, 2024 19:17
@mholttech mholttech changed the title WIP: Add remote indices feature Add remote indices feature Aug 22, 2024
@mholttech mholttech changed the title Add remote indices feature Add support for remote_indicies to elasticstack_elasticsearch_security_role & elasticstack_kibana_security_role Aug 22, 2024
@mholttech
Copy link
Copy Markdown
Contributor Author

@tobio This is ready for another review

Copy link
Copy Markdown
Member

@tobio tobio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple of minor comments. Generally this LGTM. Thanks so much for adding this! It's a decent chunk of code here. ❤️

Comment thread internal/elasticsearch/security/role.go Outdated
Comment thread internal/elasticsearch/security/role.go Outdated
Comment thread internal/kibana/role.go Outdated
Comment thread internal/models/models.go Outdated
@mholttech
Copy link
Copy Markdown
Contributor Author

Couple of minor comments. Generally this LGTM. Thanks so much for adding this! It's a decent chunk of code here. ❤️

Thanks! I've reviewed your feedback and now that i've learned how everything works and it's in a working state I'll attempt some of the suggested code cleanup in indices and remote_indices before we merge this.

mholttech and others added 2 commits August 22, 2024 21:11
@mholttech
Copy link
Copy Markdown
Contributor Author

@tobio I just pushed some code improvements based on your feedback.

Copy link
Copy Markdown
Member

@tobio tobio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This is a big chunk of code, thanks for adding this to the provider!

@tobio tobio merged commit cf53ad9 into elastic:main Aug 26, 2024
@mholttech
Copy link
Copy Markdown
Contributor Author

@tobio when will the next version be released so we can begin leveraging this?

@tobio
Copy link
Copy Markdown
Member

tobio commented Sep 10, 2024

@mholttech I'm expecting these to be released ideally later this week, but by the end of next week at the latest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Add support for remote_indices in elasticstack_elasticsearch_security_role

2 participants